-
-
Notifications
You must be signed in to change notification settings - Fork 612
feat: add support for [email protected]
#2749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… syncArchs task, bump react-native-gesture-handler
Motivation: working on #2466 right now & need these changes + this is also required to fix the failing CI Bumped versions of reanimated, gesture handler and safe-area-context in both lib and the examples :fingers_crossed: CI? Okay, seems that Android part of the CI is fixed. iOS has some other, yet undetermined problems. I do not see a regression on iOS part, however. - [x] (kinda ☝🏻) Ensured that CI passes (cherry picked from commit 3555d23)
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
I've left few remarks considering breaking the backward compatibility.
We can consider cherry picking #2730 separately instead of applying it here. Just let me know what do you prefer before we proceed.
And I have a question: have you tested these changes on fresh application already? It's fine if not, I just wanna now what is left before release.
android/src/paper/java/com/facebook/react/viewmanagers/RNSScreenContainerManagerInterface.java
Outdated
Show resolved
Hide resolved
|
We'll also need to update version compatibility table in followup PR before release. |
I think I'll try to apply it here.
With my memory of a goldfish, I am 95% sure that I did but after doing the cherrypick we probably should redo the tests as well - then I'll be 100% sure ;) |
Restoring backward compatibility for old architecture - now we require 76. 75 could work, but I've not tested this. On new architecture we require 77. It should compile on 76, however we need the fix for the removal of transitioning views that landed in 77. See here: #2596. Described mostly down below 👇🏻 in review comments. I've tested this manually on fresh RN 76 & 78 app, new/old architecture, iOS/Android. - [ ] Ensured that CI passes (cherry picked from commit b5a0f9f)
…c frameworks (#2737) ## Description It seems that '-' characters in import names are swapped for '_' when using dynamic frameworks, therefore we need to check not only for `React-RCTAppDelegate` but also for `React_RCTAppDelegate`. ## Test code and steps to reproduce Build with `USE_FRAMEWORKS=dynamic` now passes. ## Checklist - [ ] Ensured that CI passes (cherry picked from commit e3eca55)
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks solid!
## Description Apparently there are some issues with newer xcode versions on GitHub CI Following [gesture-handler lead](software-mansion/react-native-gesture-handler#3319) here. ## Changes Set `xcode-version` to `16.1` temporarily in our CI workflow definitions. ## Test code and steps to reproduce hopefully iOS-related workflows pass ## Checklist - [ ] Ensured that CI passes (cherry picked from commit 9ed3987)
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI errors seem to be unrelated to the PR itself - there are some temporary issues with reanimated 3.17.1. The flows work when tested manually.
Description
Adding support for
[email protected].Changes
syncArchstask, because updatingreact-nativeversion of the library fixed the problem with codegen,replaceNavigationBarViewsWithSnapshotOfSubviewmethod by adding a nil check:Test code and steps to reproduce
CI
Checklist